Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Select correct format after install/remove #148

Merged
merged 2 commits into from
Nov 4, 2021
Merged

Select correct format after install/remove #148

merged 2 commits into from
Nov 4, 2021

Conversation

davidmhewitt
Copy link
Member

@davidmhewitt davidmhewitt commented Nov 4, 2021 •
edited
Loading

I've broken out part of #147 for this

Fixes #83

In LocaleManager.vala:
Make sure we fall back to our alternative ways of getting the user's formats if its not set on AccountsService.

In Plug.vala:
Since the langs array is overwritten by the reload method, it doesn't make sense to add/remove stuff first.

In LocaleSetting.vala:
Since the tree store is sorted alphabetically on the human readable name of the language (which could be translated), it doesn't make sense to try and select the active one by index, because the indexes could change on sort. So we use the locale key to select the active entry.

@davidmhewitt davidmhewitt force-pushed the fix-reload-after-install branch from 33efa7f to 75862c0 Compare November 4, 2021 15:26
@davidmhewitt davidmhewitt mentioned this pull request Nov 4, 2021
Open
danirabbit
danirabbit previously approved these changes Nov 4, 2021
View reviewed changes
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable! Can we get a line in appdata? :)

@awissu
Copy link

awissu commented Nov 4, 2021

In Plug.vala:
Since the langs array is overwritten by the reload method, it doesn't make sense to add/remove stuff first.

Oh, Unfortunately, the langs array is not overwritten. I solved this problem in this way:

src/Utils.vala:

+        public static Gee.ArrayList<string>? get_installed_languages (bool? check) {
+            if (installed_languages != null && check == true) {
                 return installed_languages;
             }
 
-            installed_languages = new Gee.ArrayList<string>.wrap (Gnome.Languages.get_all_locales ());
+            installed_languages = new Gee.ArrayList<string> ();
 
+            try {
+                string output;
+                Process.spawn_command_line_sync ("localectl \"list-locales\" \"--no-pager\"", out output);
+                var locales = output.split ("\n");
+                foreach (var locale in locales) {
+                    string code;
+                    if (locale != "") {
+                        if (Gnome.Languages.parse_locale (locale, out code, null, null, null)) {
+                            if (Gnome.Languages.language_has_translations (code)) {
+                                installed_languages.add (locale);
+                            }
+                        }
+                    }
+                }
+            } catch (Error e) {
+                warning (e.message);
+            }

/src/Plug.vala:

+                langs = Utils.get_installed_languages (true);
....
+                langs.clear ();
+                langs.add_all (langpacks);
+                reload.begin ();

@davidmhewitt
Copy link
Member Author

Oh, Unfortunately, the langs array is not overwritten. I solved this problem in this way:

Ah yes, that's a slightly different issue. This is the minimum needed to fix #83.

Your changes there fix a different issue where languages are not added/removed to the sidebar on the left when they're installed/removed because we keep a cache array.

I'll open a separate issue for that and fix it in a different PR.

@danirabbit danirabbit merged commit 8c1ac83 into master Nov 4, 2021
@danirabbit danirabbit deleted the fix-reload-after-install branch November 4, 2021 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danirabbit danirabbit danirabbit approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Installing missing locales change the format value
3 participants
@davidmhewitt @awissu @danirabbit